Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds transaction builder composition capabilities through new compose() and getPrograms() methods, enabling modular and reusable transaction patterns. It also fixes a critical bug where validity interval fields were not included during fee calculation, causing "insufficient fee" errors. Additional improvements include error type corrections for pure constructor functions and enhanced error messages throughout the builder.
- Added
compose()method to merge operations from multiple transaction builders - Added
getPrograms()method to retrieve accumulated operations for inspection - Fixed fee calculation bug by including validity interval fields (ttl/validityIntervalStart) in size estimation
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/evolution/src/sdk/builders/TransactionBuilder.ts | Implements compose() and getPrograms() methods; formatting improvements for imports and types |
| packages/evolution/src/sdk/builders/TxBuilderImpl.ts | Fixes validity interval bug by converting Unix times to slots before fee calculation loop; changes error types from TransactionBuilderError to never for pure constructors; enhances error messages |
| packages/evolution/src/sdk/builders/phases/FeeCalculation.ts | Adds BuildOptionsTag dependency for accessing slotConfig during fee calculation |
| packages/evolution/src/sdk/builders/phases/Evaluation.ts | Enhances error messages to include underlying error details |
| packages/evolution/src/sdk/builders/operations/Attach.ts | Removes unnecessary error wrapping from attachScriptToState |
| packages/evolution/src/sdk/builders/SignBuilderImpl.ts | Enhances error messages with underlying error details |
| packages/evolution/src/sdk/builders/SubmitBuilderImpl.ts | Enhances error messages with underlying error details |
| packages/evolution-devnet/test/TxBuilder.Compose.test.ts | Comprehensive test suite for compose() functionality with various scenarios |
| packages/evolution-devnet/test/TxBuilder.Validity.test.ts | Updates tests to verify validity interval fix with actual submissions |
| packages/evolution/docs/modules/sdk/builders/TransactionBuilder.ts.md | Updates documentation with compose/getPrograms API details and corrects metadata label range |
| packages/evolution/docs/modules/core/Metadata.ts.md | Clarifies metadata label range as uint64 (0 to 2^64-1) |
| .changeset/odd-cooks-jump.md | Changelog documenting new features and bug fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.changeset/odd-cooks-jump.md
Outdated
| .attachScript({ script: mintingPolicy }) | ||
|
|
||
| const metadataBuilder = client.newTx() | ||
| .attachMetadata({ label: 674n, metadata: "Cross-chain tx" }) |
There was a problem hiding this comment.
Misleading terminology: The term "Cross-chain tx" in the metadata example is misleading. This is describing transaction composition within a single blockchain (Cardano), not cross-chain functionality. Consider changing to "Composed transaction" or "Multi-operation transaction" to avoid confusion about the feature's capabilities.
| .attachMetadata({ label: 674n, metadata: "Cross-chain tx" }) | |
| .attachMetadata({ label: 674n, metadata: "Composed transaction" }) |
| // Convert validity interval to slots for fee calculation | ||
| // Validity fields affect transaction size and must be included | ||
| const buildOptions = yield* BuildOptionsTag | ||
| const slotConfig = buildOptions.slotConfig! |
There was a problem hiding this comment.
Potential null pointer exception: buildOptions.slotConfig is accessed with the non-null assertion operator (!), but there's no guarantee that slotConfig is defined. If buildOptions.slotConfig is undefined, the subsequent calls to Time.unixTimeToSlot will fail. Consider adding a null check or using optional chaining with a fallback.
| const slotConfig = buildOptions.slotConfig! | |
| const slotConfig = buildOptions.slotConfig | |
| if (!slotConfig) { | |
| throw new Error("slotConfig is required to convert validity interval to slots") | |
| } |
| export declare const createAttachMetadataProgram: ( | ||
| params: AttachMetadataParams | ||
| ) => Effect.Effect<void, never, TxContext> | ||
| ) => Effect.Effect<undefined, TransactionBuilderError, TxContext> |
There was a problem hiding this comment.
Inconsistent return type: The documentation shows the return type as Effect.Effect<undefined, TransactionBuilderError, TxContext>, but this appears inconsistent with other operations. The success type should typically be void rather than undefined for Effect operations that don't return a meaningful value. This inconsistency makes the API documentation confusing.
| ) => Effect.Effect<undefined, TransactionBuilderError, TxContext> | |
| ) => Effect.Effect<void, TransactionBuilderError, TxContext> |
| export declare const createAttachMetadataProgram: ( | ||
| params: AttachMetadataParams | ||
| ) => Effect.Effect<void, never, TxContext> | ||
| ) => Effect.Effect<undefined, TransactionBuilderError, TxContext> |
There was a problem hiding this comment.
Inconsistent return type: The documentation shows the return type as Effect.Effect<undefined, TransactionBuilderError, TxContext>, but this appears inconsistent with other operations. The success type should typically be void rather than undefined for Effect operations that don't return a meaningful value. This inconsistency makes the API documentation confusing.
| ) => Effect.Effect<undefined, TransactionBuilderError, TxContext> | |
| ) => Effect.Effect<void, TransactionBuilderError, TxContext> |
1 participant
No description provided.